Skip to content

Review and analyze last two pull requests#70

Merged
MarcosBrendonDePaula merged 1 commit intomainfrom
claude/review-recent-prs-Zj9ue
Feb 27, 2026
Merged

Review and analyze last two pull requests#70
MarcosBrendonDePaula merged 1 commit intomainfrom
claude/review-recent-prs-Zj9ue

Conversation

@MarcosBrendonDePaula
Copy link
Copy Markdown
Collaborator

@MarcosBrendonDePaula MarcosBrendonDePaula commented Feb 27, 2026

Summary

Fixes 6 bugs identified during the code review of PR #68 (LiveComponent DX Enhancements) and PR #69 (Bugfix followup). All fixes include regression tests that first fail to prove the bug exists, then pass after the fix is applied.

Bugs Fixed

1. onAction cancellation leaks internal info to client (HIGH)

Before: When onAction() returns false, the error "cancelled by onAction hook" was emitted as an ERROR message to the client, revealing server-side rate-limiting/guard logic.
After: Cancelled actions no longer emit ERROR to the client. The thrown error uses a generic "was cancelled" message. If onAction itself throws, the client receives "failed pre-validation" without leaking hook details.

2. setState emits STATE_DELTA even when values are unchanged (MEDIUM)

Before: setState({ count: 0 }) on a component with count: 0 would emit a STATE_DELTA and fire onStateChange, causing unnecessary network traffic. The proxy set handler already had this check, but setState did not.
After: setState now filters to only actually-changed keys before emitting. If nothing changed, it's a no-op. Consistent with proxy behavior.

3. Silent catch {} swallows lifecycle hook errors (MEDIUM)

Before: Errors in onStateChange, onRoomJoin, and onRoomLeave were caught with empty catch {} blocks — zero feedback when developer code fails.
After: All three hooks now log errors via console.error.

4. Singleton broadcast missing userId and room metadata (MEDIUM)

Before: The singleton emit override in ComponentRegistry created LiveMessage objects without userId or room fields, breaking client-side logic that depends on these fields.
After: The override now includes userId: component.userId and room: component.room, matching the normal emit path.

5. New onClientJoin/onClientLeave lifecycle hooks for singletons (HIGH)

Before: Singleton onConnect only fired for the first client. onDisconnect fired for every client disconnect, even when others were still connected. No way to track per-connection events.
After:

  • onClientJoin(connectionId, connectionCount) fires for every new client joining a singleton (including the first)
  • onClientLeave(connectionId, connectionCount) fires for every client leaving
  • onDisconnect now only fires when the last client disconnects (semantically correct)
  • Both hooks are added to BLOCKED_ACTIONS for security

6. onAction throwing (not returning false) not properly handled

Before: If onAction threw an exception (as opposed to returning false), the raw error message was sent to the client via the generic ERROR emit.
After: onAction exceptions are caught separately and the client receives a sanitized "failed pre-validation" message instead of the raw error.

Files Changed

File +/- Description
core/types/types.ts +102/-15 Fix onAction leak, setState no-op, silent catches, add onClientJoin/Leave hooks
core/server/live/ComponentRegistry.ts +61/-2 Fix singleton broadcast metadata, add onClientJoin/Leave calls, fix onDisconnect semantics
tests/unit/core/live-component-bugfixes.test.ts +545 (new) 19 regression tests covering all 6 bugs
tests/unit/core/live-component-dx.test.ts +3/-3 Update 3 error message assertions to match new cancelled action wording

Test Plan

  • 19 new regression tests in live-component-bugfixes.test.ts
  • 76 existing DX tests pass (3 error messages updated)
  • 65 existing security tests pass (no changes)
  • Total: 160/160 tests passing across 3 LiveComponent test files

- onAction cancellation no longer emits ERROR to client (info leak)
- setState skips emit/hooks when values are unchanged (parity with proxy)
- Silent catch {} replaced with console.error in onStateChange, onRoomJoin, onRoomLeave
- Singleton broadcast now includes userId and room metadata
- Add onClientJoin/onClientLeave lifecycle hooks for singletons
- Singleton onDisconnect only fires when last client leaves
- New hooks added to BLOCKED_ACTIONS for security
- 19 regression tests covering all fixed bugs

https://claude.ai/code/session_01JEtihEZe9cThDAadXAp3Rp
@MarcosBrendonDePaula MarcosBrendonDePaula merged commit 09c02ac into main Feb 27, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants